Skip to content

zfsUnstable: disable kernel compatibility checks#145485

Closed
hmenke wants to merge 1 commit intoNixOS:masterfrom
hmenke:zfs-compat
Closed

zfsUnstable: disable kernel compatibility checks#145485
hmenke wants to merge 1 commit intoNixOS:masterfrom
hmenke:zfs-compat

Conversation

@hmenke
Copy link
Copy Markdown
Member

@hmenke hmenke commented Nov 11, 2021

Motivation for this change

With this change zfsUnstable has all kernel compatibility checks disabled. This allows using zfsUnstable with kernels that have not been marked compatible by upstream.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

With this change zfsUnstable has all kernel compatibility checks
disabled. This allows using zfsUnstable with kernels that have not been
marked compatible by upstream.
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` labels Nov 11, 2021
@hmenke hmenke mentioned this pull request Nov 11, 2021
12 tasks
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 11, 2021
@hmenke hmenke marked this pull request as ready for review November 11, 2021 16:51
@Ma27
Copy link
Copy Markdown
Member

Ma27 commented Nov 12, 2021

Can't we put this behavior behind a flag? This seems pretty risky tbh.

@hmenke
Copy link
Copy Markdown
Member Author

hmenke commented Nov 12, 2021

What kind of flag are you thinking about?

@hmenke
Copy link
Copy Markdown
Member Author

hmenke commented Nov 14, 2021

@Ma27 This behavior is already behind the flag boot.zfs.enableUnstable which I think is sufficient, especially in conjunction with the warning message in the option text and the documented change in the release notes.

@Ma27
Copy link
Copy Markdown
Member

Ma27 commented Nov 14, 2021

Considering how zfsUnstable is used (e.g. it points to stable 2.1 on 21.05 because zfsStable is still 2.0.5 which doesn't support newer kenrels) I disagree.

The version will have already passed an extensive test suite, but it is more likely to hit an undiscovered bug compared to running a released version of ZFS on Linux.

Throwing away all compatibility checks IMHO contradicts with this.

What kind of flag are you thinking about?

Idk, linuxPackages.zfs.override { disableCompatibilityCheck = true; } for instance?

@hmenke
Copy link
Copy Markdown
Member Author

hmenke commented Nov 14, 2021

Idk, linuxPackages.zfs.override { disableCompatibilityCheck = true; } for instance?

Where should this go inside configuration.nix then? This requires a rather complicated overlay, plus it will probably trigger a rebuild.

@hmenke
Copy link
Copy Markdown
Member Author

hmenke commented Nov 14, 2021

This is the minimal overlay that you need to override something in zfsUnstable, because you need to override both, the kernel module and the userspace tools.

{ config, pkgs, ... }:

{
  boot.kernelPackages = pkgs.linuxPackages_latest;
  boot.zfs.enableUnstable = true;
  nixpkgs.overlays = [
    (self: super: {
      linuxPackages_latest = super.linuxPackages_latest.extend (linuxPackagesSelf: linuxPackagesSuper: {
        zfsUnstable = linuxPackagesSuper.zfsUnstable.override {
          #...
        };
      });

      zfsUnstable = super.zfsUnstable.override {
        # ...
      };
    })
  ];
}

@hmenke
Copy link
Copy Markdown
Member Author

hmenke commented Nov 14, 2021

Considering how zfsUnstable is used (e.g. it points to stable 2.1 on 21.05 because zfsStable is still 2.0.5 which doesn't support newer kenrels) I disagree.

I disagree with this statement. The way I see zfsUnstable being used is that people who want to run the latest kernel are forced to switch to zfsUnstable for that support. However, upstream is often rather slow with marking their versions (including release candidates) as compatible with the latest kernel versions. This often results in a dilemma which we are in again right now, because Linux 5.15 has been released but there is no release or rc of OpenZFS that is marked as compatible with 5.15. By trying out it was discovered that the current ZFS 2.1.1 is probably compatible with 5.15 anyway, so it was proposed to just bump the version check #145458. In my opinion this is inconsequential and wrong, because now the compatibility check is complete bogus and the user is not even warned about it.

@Ma27
Copy link
Copy Markdown
Member

Ma27 commented Nov 17, 2021

This is the minimal overlay that you need to override something in zfsUnstable, because you need to override both, the kernel module and the userspace tools.

I'd be fine with this if it gets documented somewhere.

In my opinion this is inconsequential and wrong, because now the compatibility check is complete bogus and the user is not even warned about it.

Well, if users want to use zfs against a kernel that's (not yet) officially supported, they are free to do so via e.g. an overlay as you demonstrated it above. But we as a distro should adhere to what upstream says IMHO.

@hmenke
Copy link
Copy Markdown
Member Author

hmenke commented Nov 17, 2021

Well, if users want to use zfs against a kernel that's (not yet) officially supported, they are free to do so via e.g. an overlay as you demonstrated it above.

That used to be my stance as well but recently every two weeks or so someone shows up whining that they cannot use the latest kernel with ZFS.

@ncfavier
Copy link
Copy Markdown
Member

FWIW, I encountered this issue while trying to build an ISO image with the installation-cd-minimal-new-kernel profile, since the base profile enables ZFS support.

@Ma27
Copy link
Copy Markdown
Member

Ma27 commented Nov 17, 2021

whining that they cannot use the latest kernel with ZFS.

tbh that's better than people whining about fs issues because NixOS didn't stop them from installing zfs with an incompatible kernel. Yes, it may be fine in most of the cases if only the compatible version-range has to be updated, but that's not always the case and we as a distro shouldn't make guarantees upstream can't make (yet). I have to admit that I didn't know of the overlay-approach you posted above, so I'd suggest we put this into the manual and point everyone to it if they're unhappy.

@SuperSandro2000 SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 20, 2021
@hmenke hmenke closed this Dec 13, 2021
@hmenke hmenke deleted the zfs-compat branch July 11, 2022 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants